Bug 190570

Taking over Alexey's JSF and Spring issues.

Can you please take a few performance snapshots for this and attach them here? Check http://blogs.sun.com/netbeansphp/entry/is_the_ide_slow_send for instructions on how take a snapshot (it's written for PHP apps but the process is exactly the same for Java apps):

1. enable the self-profile button
2. start profiling
3. invoke code completion in a spring config file
4. stop profiling
5. save the snapshot

Please repeat this 2-3 times and attach the snapshots here then.

I was able to reproduce this myself - this doesn't happen with all Spring 3 projects, but in some cases the IDE indeed downloads the schema every time. Passing to xml/schema for evaluation. 

You can reproduce e.g. with the travel sample:

1. svn co https://src.springframework.org/svn/spring-samples/travel
2. open travel/src/main/resources/META-INF/spring/data.xml
3. invoke CC => I took snapshots and the time is spent in o.n.m.xml.schema.completion.CompletionQuery

This is not specific only for Windows, I have the same issue in OpenSuSE 11.3 x86_64

Created attachment 107021 [details]
snapshot

There is one additional issue with XML retriever :
XML retriever uses Project directory for cache downloaded files ( schemes, ... ).
In case of file is outside of Project XML Retriever is failed in the
org.netbeans.modules.xml.retriever.impl.Util.retrieveAndCache() method.

As result files are downloaded each time when code completion is invoked for file 
that is out Project ( my nps file is for external file ).

But this is minor issue because external files is rarely used.

THIS issue is valid only for Maven projects.
All works fine for ANT based project.
The schema file is downloaded only at the first time when CC is invoked per 
project for ANT based project.
The cache dir is nbproject\private\retriever.

But this is not valid for Maven projects.
The same method org.netbeans.modules.xml.retriever.impl.Util.retrieveAndCache()
fails after string
FileObject cacheFO = FileUtil.toFileObject(FileUtil.normalizeFile(cacheFile));
because the cacheFO is outside of the Project directory structure
and it has no respective FileObject.
The described behavior is consequence of return value of the CacheDirectoryProvider.getCacheDirectory().
For ANT based project it is nbproject/privite but for Maven project it is 
somewhere inside of var\cache\mavencachedirs\ in the userdir .

There are could be options :
- the easiest way is either use cache dir for maven projects under project 
directory . In this case FileObject will be not null for such dir.
- use the same cache dir but it should has corresponding not null FileObject.
- Rewrite XML retriever implementation: do not use FileObject but just File.

The latter option is too risky for now. I would suggest to fix it on the 
Maven side. 
I don't know the original functional functionality but retriever should use 
some global Catalog ( not per Project ) from my point of view.
Now each project has its own Catalog and it leads to download the same schema
files for different project at the first time CC usage.
But there could be a reasons why the current approach is chosen instead of 
one global catalog.

(In reply to comment #6)
> use cache dir for maven projects under project directory

No, there is no equivalent location for Maven projects.

> - use the same cache dir but it should has corresponding not null FileObject.

AFAIK FileUtil.toFileObject ought to return a non-null value when passed any existing File on disk. Perhaps the client code in an XML module did not create the dir?

> - Rewrite XML retriever implementation: do not use FileObject but just File.

Seems like the best approach.

> there could be a reasons why the current approach is chosen instead of 
> one global catalog.

Afraid I do not know the background for this either.

You are right actually about FileObject.
It's my fault: I've noticed that the file path is incorrect.
This is why FileObject is null.
There are a number of relativize() method which calculate the path relative to 
the project. The implementation if this method is incorrect.
This is the reason of the bug.

changeset:   191062:bc7abf38dc30

Thanks Denis and Jesse. Adding 70_HR_FIX_CANDIDATE - Denis, can you please transplant to 7.0 following the HR process?

OK.

Jesse, could you please review the change for integrating it into 7.0 branch ?

Not familiar with this code. Looking at the patch:

1. Shouldn't RetrieverTest.testRelativize test this change? (Not sure why this test case is not in a UtilitiesTest.)

2. Is there something wrong with the standard URI.relativize that you need this rather complex and undertested method? (Compare what is checked in org.netbeans.spi.project.support.ant.PropertyUtilsTest.testRelativizeFile.) I know URI.relativize does not produce ../ sequences when it could.

(In reply to comment #13)
> Not familiar with this code. Looking at the patch:
> 
> 1. Shouldn't RetrieverTest.testRelativize test this change? (Not sure why this
> test case is not in a UtilitiesTest.)
changeset:   191110:bc53aa1ca2b7
> 
> 2. Is there something wrong with the standard URI.relativize that you need this
> rather complex and undertested method? (Compare what is checked in
> org.netbeans.spi.project.support.ant.PropertyUtilsTest.testRelativizeFile.) I
> know URI.relativize does not produce ../ sequences when it could.
Exactly: URI.relativize does not produce ../ sequences .
I don't know is it really needed but relativize() method is widely used in the
Retriever module and it is hard to investigate its necessity and resulting format.
The fix which I've introduced works fine for Maven case when cache dir is outside
of the Project dir and it doesn't change behavior in case of Ant based project.

(In reply to comment #14)
> changeset:   191110:bc53aa1ca2b7

That would be: web-main #bc53aa1ca2b7

For best hyperlinking, see: http://wiki.netbeans.org/BrowserTools#Link_to_Mercurial_Changesets

> URI.relativize does not produce ../ sequences.
> The fix which I've introduced works fine for Maven case when cache dir is
> outside of the Project dir

The question is whether ../ sequences are *desirable* in this case. Not sure what is being relativized to what, but if you are storing e.g.

  ../../../../../../../../home/jglick/.netbeans/dev/var/maven/something/

rather than e.g.

  file:/home/jglick/.netbeans/dev/var/maven/something/

then it is counterproductive.

I believe one of the cases when relativization is used is when creating a local schema catalog. Retreiver downloads schema files, saves them locally, and then creates relative links among related schemas. It also puts them in a local catalog.xml. These schemas may be used by the project's build script (at least for Ant projects), so they need to be stored relatively to the project sources.

(In reply to comment #15)
> (In reply to comment #14)
> > changeset:   191110:bc53aa1ca2b7
> 
> That would be: web-main #bc53aa1ca2b7
> 
> For best hyperlinking, see:
> http://wiki.netbeans.org/BrowserTools#Link_to_Mercurial_Changesets
OK.
> 
> > URI.relativize does not produce ../ sequences.
> > The fix which I've introduced works fine for Maven case when cache dir is
> > outside of the Project dir
> 
> The question is whether ../ sequences are *desirable* in this case. Not sure
> what is being relativized to what, but if you are storing e.g.
> 
>   ../../../../../../../../home/jglick/.netbeans/dev/var/maven/something/
> 
> rather than e.g.
> 
>   file:/home/jglick/.netbeans/dev/var/maven/something/
> 
> then it is counterproductive.
I don't store anything. This is not my responsibility and not mine functionality.
I've already said : I don't know is it really needed but relativize() 
method is widely used in the Retriever module and it is hard to investigate its necessity and resulting format.
Relativized URI is used inside code logic not only for storing it into the file.
I've introduced the very minor change to be sure that nothing other broken.
I'm pretty sure that codefreeze is not time for more serious changes.

No problem if you object against suggestion of the fix: it will not be
integrated into the 7.0 branch and will not be fixed for 7.0.

(In reply to comment #17)
> I've introduced the very minor change to be sure that nothing other broken.
> I'm pretty sure that codefreeze is not time for more serious changes.

Agreed, my second comment applied more to ongoing development. Since there is at least some test coverage of the change, it seems safe for 7.0.

Thanks.

Integrated into 'main-golden', will be available in build *201103170400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/bc7abf38dc30
User: Denis Anisimov <ads@netbeans.org>
Log: Fix for BZ#190570 - Spring 3 Slow auto completion

Is there any reason this issue hasn't been integrated into release70 yet ? Please do it ASAP, so we can test it before RC1 ! Thanks in advance.

I'm sorry Marian but I've tried to follow the procedure :
http://wiki.netbeans.org/NB70HighResistance
And there is still no QA approval.

OK, I will fix into release70.

changeset:   195382:d60f89553b5d
branch:      release70

I tried test case described in comment #3 in NetBeans 6.9.1 and I haven't seen any slowness. The same in 7.0 build 201103250000 (WindowsXP, JDK6.0u24). If you have any particular test case, I can try, please, let me know.

Test case from comment 3 is good enough to check this bug against.

Then OK for me.

Created attachment 109693 [details]
Spring Slow Autocompletion

Hello, 
there seems to be a regression on nb 7.0, at least for me on Netbeans 7.0 just reinstalled with clean userdir & fully updated, Ubuntu 11.04 32 bit, Sun Java 6 SDK.

It still happens for Maven webapps only, Spring autocompletion works correctly for ant webapps; on the contrary, If i try using autocompletion on a maven webapp, I either get "Downloading..." or "Please wait..." message at EVERY SINGLE ctrl+click I employ, then I need something between 10 and 30 seconds for the suggestions to appear.

Such messages appear on ant webapps as well, but the first time only; it seems a not-caching issue. I tried clean & build multiple times to force artifact download, still no luck.

In order to reproduce:

- Install Netbeans 7.0
- create a brand new maven webapp project (ee 5 or 6, it's the same)
- create a Spring XML configuration file, and add Spring XML Framework
- try autocompleting <bean> itself or whatever attribute you like in the resulting xml.

Ok, assigning to Martin as the Spring owner, also cc David regarding the Maven aspect.

Reproduced, I'm able to confirm that using mentioned steps or just by adding Spring via Web Project Framework and editing Spring config xml.

Created attachment 109723 [details]
patch

I didn't investigated deeply why this stopped to working.

Anyway, by retrieving resources it's still looking for cached catalog inside project dir (RetrieverImpl:127) which can't work for Maven projects. There is raised NPE then which comes to empty catch block. :/ 

Attached patch shouldn't make anything worse, I suppose. If the project isn't null previous solution is taken, otherwise it takes destinationDir as an appropriate place for storing/loading catalog.

But whole XML catalog caching is quite magic to me, so could anybody take a look on/review attached patch please?

Denis, you looked into this area before - could you please review Martin's patch?

(In reply to comment #32)
> Denis, you looked into this area before - could you please review Martin's
> patch?

OK.

(In reply to comment #31)
> Created an attachment (id=109723) [details]
> patch
> 
> I didn't investigated deeply why this stopped to working.
That's the problem of the patch.
> 
> Anyway, by retrieving resources it's still looking for cached catalog inside
> project dir (RetrieverImpl:127) which can't work for Maven projects. There is
> raised NPE then which comes to empty catch block. :/ 
Not exactly.
The code tries to resolve "relative" path respectively project folder.
It doesn't mean that "relative" path should be exactly inside project dir.
In the latter case resolution result will be full path to the file.

The problem which I've found and resolved in my previous fix had exactly 
this scenario.
catalog.xml file for Maven projects is placed somewhere inside netbens user directory ( where config and var folders with common NB cache ) not inside project folder.
So I think the problem is inside resolving "relative" path respectively project directory.

By the way , please clarify exactly this :
>which can't work for Maven projects. There is
> raised NPE then which comes to empty catch block.
Please attach this NPE.
> 
> Attached patch shouldn't make anything worse, I suppose. If the project isn't
> null previous solution is taken, otherwise it takes destinationDir as an
> appropriate place for storing/loading catalog.
This issue is not about problem with CC outside of project.
This issue is about problem of CC inside Maven project.
So your fix could just help avoid a problem with CC outside of project ( "could" because I care about the putting catalog.xml file inside destinationDir which value I don't know. Probably some common cache dir should be chosen inside of netbeans user directory avoiding duplication ).
But it definitely doesn't fix the original problem with Maven projects.
> 
> But whole XML catalog caching is quite magic to me, so could anybody take a
> look on/review attached patch please?

So please do a debugging the problem and find the path where catalog.xml *should* be and why it doesn't reused with the second attempt of CC.

By the way : is it really an issue ? I mean my previous fix was about pretty same scenario. And it was verified. 
So there could be no a problem at all : the very first usage of CC needs to retrieve  XSD file from external location. And this resource consuming task.
That's OK. This is not a problem.
The second usage of CC shouldn't retrieve the schema file. If it does then this is really a problem.
Please recheck this.

(In reply to comment #34)
> (In reply to comment #31)
> > Created an attachment (id=109723) [details] [details]
> > patch
> > 
> > I didn't investigated deeply why this stopped to working.
> That's the problem of the patch.

With this sentence I meant rather I wasn't looking for place where the regression happened. If it was regression, it's also possible that it wasn't just reproduced - because the same happened to me as well before (reproducible just with some UDs).

> > 
> > Anyway, by retrieving resources it's still looking for cached catalog inside
> > project dir (RetrieverImpl:127) which can't work for Maven projects. There is
> > raised NPE then which comes to empty catch block. :/ 
> Not exactly.
> The code tries to resolve "relative" path respectively project folder.
> It doesn't mean that "relative" path should be exactly inside project dir.
> In the latter case resolution result will be full path to the file.
> 

Actually, it isn't about the relative path as you was fixing before. The relative path is ok. But the problem is in RetrieverImpl:126 - sorry one line above than I wrote before. In context: 
Project prj = FileOwnerQuery.getOwner(destinationDir); 
- destination dir isn't dir inside project for Maven
FileObject prjRtFO = prj.getProjectDirectory();
- throws NPE which is catched with empty catch block in CatalogModelImpl:454
So the caching of the catalog isn't finished properly!

> The problem which I've found and resolved in my previous fix had exactly 
> this scenario.
> catalog.xml file for Maven projects is placed somewhere inside netbens user
> directory ( where config and var folders with common NB cache ) not inside
> project folder.
> So I think the problem is inside resolving "relative" path respectively project
> directory.

I really don't think so, all paths are valid. Problem is in the NPE because looking for project in destinationDir.

> 
> By the way , please clarify exactly this :
> >which can't work for Maven projects. There is
> > raised NPE then which comes to empty catch block.
> Please attach this NPE.

I fully described where the NPE happened and also where it's swallow.

> > 
> > Attached patch shouldn't make anything worse, I suppose. If the project isn't
> > null previous solution is taken, otherwise it takes destinationDir as an
> > appropriate place for storing/loading catalog.
> This issue is not about problem with CC outside of project.
> This issue is about problem of CC inside Maven project.
> So your fix could just help avoid a problem with CC outside of project (
> "could" because I care about the putting catalog.xml file inside destinationDir
> which value I don't know. Probably some common cache dir should be chosen
> inside of netbeans user directory avoiding duplication ).
> But it definitely doesn't fix the original problem with Maven projects.

I know that the problem is CC inside the project. Did you tried debug on that place? BTW, I found somewhere around this comment:
@param destinationDir   A folder inside a NB project (ONLY) to which the retrieved resource will be copied to. All retrieved imported/included resources will be copied relative to this directory.

It means, it wasn't prepared for storing catalog outside the project directory. So there is not handled the case of Maven. Here my patch should help because in cases of Maven directory appropriate dir is chosen for storing the catalog (and the condition prevent mentioned NPEs).

> > 
> > But whole XML catalog caching is quite magic to me, so could anybody take a
> > look on/review attached patch please?
> 
> So please do a debugging the problem and find the path where catalog.xml
> *should* be and why it doesn't reused with the second attempt of CC.
> 
> By the way : is it really an issue ? I mean my previous fix was about pretty
> same scenario. And it was verified. 
> So there could be no a problem at all : the very first usage of CC needs to
> retrieve  XSD file from external location. And this resource consuming task.
> That's OK. This is not a problem.
> The second usage of CC shouldn't retrieve the schema file. If it does then this
> is really a problem.
> Please recheck this.

Right it was verified, but it doesn't mean that there couldn't be a regression or that it wasn't just reproduced because of any another reason. BTW, I tried it with clean userdir and it worked as well because it didn't come to the problematic method. So hard to say what has to be done to go thru patched way but I can say, that with my userdir I'm able to reproduce the same problem as the user and the patch I proposed really helped.

As you mentioned, the very first usage should retrieve XSD - it happens because of some fallback in XML, but it isn't stored on appropriate place (NPE problem) so the second usage goes thru the fallback again.

OK, I got the point.
I can't reproduce the issue because of line 208 of org.netbeans.modules.xml.retriever.catalog.impl.CatalogModelImpl 
ModelSource rms = getModelSourceFromSystemWideCatalog(locationURI, modelSourceOfSourceDocument);
I don't know how this catalog has been initialized with downloaded schema file but result is not null for me.

Go back to the issue.
Yes, you are right. There is NPE because project is null. 
Project prj = FileOwnerQuery.getOwner(destinationDir);
desitinationDir is outside of project directory so prj is null.

I'm OK with your fix. But could you please check that the following change work/doesn't work:
instead of line:
if(!relativePathToCatalogFile.isAbsolute()){
replace to :
if(!relativePathToCatalogFile.isAbsolute() || prj==null ){

Sorry , the last line should be
if(!relativePathToCatalogFile.isAbsolute() && prj!=null ){

(In reply to comment #36)
> OK, I got the point.
> I can't reproduce the issue because of line 208 of
> org.netbeans.modules.xml.retriever.catalog.impl.CatalogModelImpl 
> ModelSource rms = getModelSourceFromSystemWideCatalog(locationURI,
> modelSourceOfSourceDocument);
> I don't know how this catalog has been initialized with downloaded schema file
> but result is not null for me.

Ah so. As was mentioned, XML catalog caching is magic ;) - no idea from me as well.

> 
> Go back to the issue.
> Yes, you are right. There is NPE because project is null. 
> Project prj = FileOwnerQuery.getOwner(destinationDir);
> desitinationDir is outside of project directory so prj is null.
> 
> I'm OK with your fix. But could you please check that the following change
> work/doesn't work:
> instead of line:
> if(!relativePathToCatalogFile.isAbsolute()){
> replace to :
> if(!relativePathToCatalogFile.isAbsolute() || prj==null ){

Sure, I already tried that by writing the patch and it didn't work because there will happen then something like: new File(<relative_uri>)

Thanks a lot for the review.

(In reply to comment #35)
> Project prj = FileOwnerQuery.getOwner(destinationDir); 
> - destination dir isn't dir inside project for Maven
> FileObject prjRtFO = prj.getProjectDirectory();

By the way, see bug #200634 which may be related to this code.

Good point. I'm going to integrate the patch tomorrow so before including that into HG I will try out your case from issue #200634 if I wouldn't be able to reproduce that and if it wouldn't go thru the patched method (if the patch couldn't be still improved). Thanks.

I took a look also why happens #200634 and I have to say that this patch doesn't make anything worse or better. I will comment on that more into #200634 to keep it on one place.

Patch integrated. Fixed in web-main #16810a4dc63d.

Integrated into 'main-golden'
Changeset: http://hg.netbeans.org/main-golden/rev/16810a4dc63d
User: Martin Fousek <marfous@netbeans.org>
Log: #190570 - Spring 3 Slow auto completion

I tried test case in comment #28 and I can't see any slowness of autocompletion in Spring config file in 7.0, 7.0.1 and dev builds.

Product Version: NetBeans IDE Dev (Build 201108170601)
Java: 1.7.0; Java HotSpot(TM) Client VM 21.0-b17
System: Windows XP version 5.1 running on x86; Cp1250; en_US (nb)

Transplanted into release701_fixes branch:
http://hg.netbeans.org/releases/rev/3a15555a2b18
http://hg.netbeans.org/releases/rev/c99a6d267c91

Just a doubt: has this been released to the general public? I've got a fully updated fresh install of Netbeans 7.0.1 and the problem is still there.

Not yet, this will be a part of the patch we plan to publish next week....