Bug 171330

Created attachment 86976 [details]
Test to count the number of touches after enter in editor

The highest number of disk touches seems to come from AddOverrideAnnotation currently (through getSourceLevel method).

Fixed, source level can now by obtained through CompilationInfo, which should eliminate unneeded touches. Hope it helps, unfortunately the test does not 
run on mac properly, causes an error (java.lang.AssertionError: No files touched exp: 0 was: 2).
---
http://hg.netbeans.org/jet-main/rev/b097452d3cd3

Integrated into 'main-golden', will be available in build *200909080527* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/b097452d3cd3
User: Max Sauer <msauer@netbeans.org>
Log: #171330: Enter in Java editor generates ~220 disk touches

Improved. But there is still room for making it better. I have modified the page 
http://wiki.netbeans.org/FitnessWithoutTouches
with my current results. There is still over 40 of touches, some of them from sidebars, but some of them still request 
the source level. Please eliminate more.

OK. Not so serious now imho => P3.

Created attachment 87338 [details]
New patch that yields those ~50 touches

I am not saying this all needs to be fixed in java.editor. The stacktraces show that the Java editor can improve 
especially in getSourceLevel queries. Please evaluate this, the sooner the better.

I am more worried by other kinds of touches. Why these need to happen (this is not maven project, neither cnd or 
mobility one, so this all seems quite unnecessary):
    org.netbeans.modules.cnd.makeproject.MakeProjectFileOwnerQuery.getOwner(MakeProjectFileOwnerQuery.java:71)
    org.netbeans.api.project.FileOwnerQuery.getOwner(FileOwnerQuery.java:101)
    org.netbeans.modules.mobility.project.J2MEProjectUtils.getProjectForDocument(J2MEProjectUtils.java:118)
    org.netbeans.modules.mobility.editor.PPCompletionProvider.getAutoQueryTypes(PPCompletionProvider.java:114)
    org.netbeans.modules.editor.completion.CompletionImpl.insertUpdate(CompletionImpl.java:303)
    org.netbeans.lib.editor.util.swing.PriorityDocumentListenerList.insertUpdate(PriorityDocumentListenerList.java:79)
or
    org.netbeans.modules.maven.queries.MavenFileOwnerQueryImpl.getOwner(MavenFileOwnerQueryImpl.java:196)
    org.netbeans.api.project.FileOwnerQuery.getOwner(FileOwnerQuery.java:101)
    org.netbeans.modules.mobility.project.J2MEProjectUtils.getProjectForDocument(J2MEProjectUtils.java:118)
    org.netbeans.modules.mobility.editor.PPCompletionProvider.getAutoQueryTypes(PPCompletionProvider.java:114)

Because of these alien touches I want to keep this higher priority bug. Please evaluate and fix what you can in 
java.editor and let's pass the bug to mobility.

Took a brief look at the code - probably 
org.netbeans.modules.mobility.project.J2MEProjectUtils.getProjectForDocument(J2MEProjectUtils.java:118):

        J2MEProjectUtilitiesProvider utils = Lookup.getDefault().lookup(J2MEProjectUtilitiesProvider.class);
        if (utils == null){
            return null;
        }
        final FileObject fo = utils.getFileObjectForDocument(doc);
        if (fo != null)
            return FileOwnerQuery.getOwner(fo);

Probably either the implementation of J2MEProjectUtilitiesProvider.getFileObjectForDocument (which I could not find), or
this method could detect if the document belongs to a Mobility project somehow, and return null if not, without invoking
FileOwnerQuery (or saving that as a last resort, needed only if it really is a J2ME project).

However, unless there is some object from mobility.editor (J2MEDataObject?  But that is probably not used for all .java
files under a J2ME project - not sure) which can be tested for against the Document, to make the negative test fast, the
code will probably need to walk the parent tree of the FileObject and do *something* that touches the disk, to determine
if it is looking at a J2ME project.  I'm assuming that this code can simply return null if the project is not a mobility
project, and that there is probably a way to detect that situation based on what is already in memory, so that the
negative test does not do I/O.  Petr?

http://hg.netbeans.org/jet-main/rev/3c8315b39f5a should eliminate disk touches originating in getSourceLevel queries. Reassigning to mobility.

There is few calls via
    org.netbeans.modules.mobility.project.J2MEProjectUtils.getProjectForDocument(J2MEProjectUtils.java:118)
    org.netbeans.modules.mobility.editor.PPCompletionProvider.getAutoQueryTypes(PPCompletionProvider.java:114)
what could be done is to remember the document -> project mapping in some cache (document property, maybe) and query 
just once. The test measures second enter, so if you store this mapping during first enter, your code will do not 
additional touches the second time.

> remember the document -> project mapping in some cache

That would work;  however, I'd like to hear from Petr S. first if there is some in-memory data that can determine if we
even need to call FileOwnerQuery, before we implement a cache.  

*If* PPCompletionProvider.getAutoQueryTypes() should return no result if it is invoked against a file that does not
belong to a mobility project, and *if* we can determine from the Document or the FileObject for the document that it
does not belong to a mobility project (J2MEDataObject? some Document property? Something in the FileObject's
DataObject's Lookup?), we can simply skip the call to FileOwnerQuery completely.

Later, if there is too much I/O happening for *legitimate* calls (i.e. it really is a mobility project), we can
implement a cache if necessary.

Petr, I don't really know what PPCompletionProvider does or if there is a way to know from the Document or FileObject if
we can reliably decide if we can return nothing from PPCompletionProvider.getAutoQueryTypes(), so probably you should
take this from here.

In case of AddOverrideAnnotation, is this still connected with Mobility? Should we split the issue?

If you wish, feel free to report some separate subissues and keep this as umbrella. But I am updating the list of 
touches on http://wiki.netbeans.org/FitnessWithoutTouches so I personally don't see much need for more issue. Also 
this issue somehow captures the main objective: typing in Java editor causes huge and unnecessary I/O storm.

psuchomel: There are no more touches from AddOverrideAnnotation anymore, please see the test output or wiki page.

Integrated into 'main-golden', will be available in build *200909111401* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/3c8315b39f5a
User: Max Sauer <msauer@netbeans.org>
Log: #171330: Enter in Java editor generates 49 disk touches

I've updated the page with new results. Right now we are down to 31 touches:
http://wiki.netbeans.org/FitnessWithoutTouches
The project queries are now gone (thanks to fixes in JavaSource level, probably). The remaining issue involve 
mercurial (because we are under NetBeans Hg root), DiffSideBar and CloneableEditorSupport. Passing to Mercurial guys 
for evaluation. Lowering priority as there was some progress and the current results are influenced by test setup.

Enabling test: ergonomics#017889e557fe

Looks like a fix for issue 126156 such as Jesse and I suggest might eliminate some of the touches as a side-effect,
although it would be non-trivial to implement.

mercurial cache fix: cdev #134821f9889c
diffsidebar fix: cdev #289075e6a9e7

file access 20 times in mecurial, most of them from: 

Mercurial.getRepositoryRoot() - 10
fixed by extending the already existent caching at that place, so that file.io significantly reduced. should also have
effect in other scenarios as the method is frequently called from many places - #22d6f3114be6

StatusCache - 8 
most of them triggered by diffsidebar - fixed by ovrabec

reassigning to CloneableEditorSupport for further evaluation

Integrated into 'main-golden', will be available in build *200909181401* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/017889e557fe
User: Jaroslav Tulach <jtulach@netbeans.org>
Log: #171330: Enabling test to ensure that the number of disk touches after enter is not significantly increase over our current state (~31)

31 still seems like about 31 too many... The user pressing a key does not imply that anything has changed on disk; 
therefore if the IDE is touching the disk, something is still doing useless work.

I have fixed the test in ergonomics#bb5d5057a474 to eliminate unrelated touches. Now we are down to two. Imho caused 
by unnecessary conversion of FileObject to URI in parsing API and java.source. See 
http://wiki.netbeans.org/FitnessWithoutTouches
Can these two be eliminated as well?

Hm, I guess you want me to pass the FileObject embedded in the Indexable directly and not convert it to and from URL. I
do not think this is feasible - there is intentionally no getFileObject method on the Indexable for performance reasons.
Adding one would require a full API review - the architectural impact of such a change is not-obvious and such change is
controversial, and so the fasttrack review cannot be used for it. I do not think the method would be accepted in the
review anyway, as it would likely be massively abused and would bound us to use of FileObjects for indexing (preventing
us from using j.l.File forever).

An alternative solution is to change PersistentClassIndex to work over URL and add a URL cache into FileObject (ideally,
the cache could be shared even by the URLMapper, for the benefit of all IDE, but I won't require that for now). This
would solve these two accesses.

I have a prototype of the change in the PersistentClassIndex, but I cannot test it properly, as your change is not part
of jet-main yet.

I have changed the PersistentClassIndex to keep the URL and convert it to FileObject/JavaSource when needed, instead of
converting it eagerly:
http://hg.netbeans.org/jet-main/rev/6f49fa94772a

For the remaining disk touch (call to FileObject.getURL()), I do not think this can be actually fixed in
java.source+parsing.api, as noted above. Would be better, IMO, to cache the result on filesystem level, so that only the
first call to getURL would touch the disk, and the subsequent calls would simply return the cached value. This may be
helpful for many parts of the IDE, not only this single usecase.

I eliminated disk touch in FileObject.getURL() by overwriting file.toURI(). We already know whether given file is folder
or not.
core-main #71d55a1c829c

Integrated into 'main-golden', will be available in build *200910150201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/71d55a1c829c
User: Jiri Skrivanek <jskrivanek@netbeans.org>
Log: #171330 - Eliminate disk touch by overwriting file.toURI() because we know whether given file is folder or not.