This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 268744 - Implement another way of caching of File instances in NetBeans
Summary: Implement another way of caching of File instances in NetBeans
Status: NEW
Alias: None
Product: ide
Classification: Unclassified
Component: Performance (show other bugs)
Version: Dev
Hardware: PC Windows 7
: P3 normal with 1 vote (vote)
Assignee: Tomas Hurka
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-30 12:22 UTC by NukemBy
Modified: 2016-10-30 12:22 UTC (History)
0 users

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
CachedFile+CachedFuri+LruCache.diff (258.45 KB, application/octet-stream)
2016-10-30 12:22 UTC, NukemBy
Details

Note You need to log in before you can comment on or make changes to this bug.
Description NukemBy 2016-10-30 12:22:09 UTC
Created attachment 162688 [details]
CachedFile+CachedFuri+LruCache.diff

Hello, lately, being rather disappointed by performance of NetBeans (extrenly slow background-scan and other various file-related operation) which did not improve in last Release 8.2 I've self-profiled these operations and noticed that rather much is spent in listing and reading attributes of files. I'm a 'windows 7' user and this problem appears to be known-yet-not-solved issue for a long time (https://netbeans.org/bugzilla/show_bug.cgi?id=65135). According to my metrics - reading file attributes on linux is 10 times quicker than on windows, therefore that problem is not that critical there, but there are some algorithms in-place in NetBeans were event 10-time-quicker file access still causes significant latency.

That's why I've made an attempt to implement radically different approach to access file attributes - global cache of File instances.

How it works:

1. There is LRU cache of File instances. Instead of `file = new File(...)` client should use `file = CachedFile.get(...)`.

   This will return 'singleton' instance of file, which also stores 'BasicFileAttributes' (initialized during calls like file.IsDirectory() and parent.listFiles()).
   These attributese are cached for rather long period and are reset in explicit file operations like 'renameTo()' or when 'file change' event is received from File notifier (already provided by NetBeans). Caching also supports File.listFiles() - also very frequent and time-consuming operation.
   
2. Adoption required

    * changing of `file = new File(...)` -> `file = CachedFile.get(...)`
    * changing implementation of 'BaseFileObj' and in some other places to immediatelly drop cached attributes when changes are done inside NetBeans over files covered by instanceof of CachedFile

3. Known issues -

    Because sub-classed CachedFile has some additional properties and is not binary compatible with File there can be problems in external world (3rd party tools and libs), e.g.
    - Gradle plugin - serialized CachedFile cannot be deserialized in target process because CachedFile is not known there
    - Maven Embedder - it tries to itrospect object included into project model (POM file) and CachedFile with additional properties inside causes endless loop there (i would say this is bug in MAVEN itself)
    
    For such cases CachedFile should be converted back to File when invoking some 3rd party code. 

    
Otherwise - it works and seems to work pretty stable. I'm using my custom build for last 2-3 weeks.

Performance improvement in some cases very significant, especially in cases when algotithms of file access are not 'optimal' by design. For example latency described here https://netbeans.org/bugzilla/show_bug.cgi?id=250470 - is decreased from 35-40 secs to around 15 sec - efficient caching eliminated 20+ secs of file-access operations and the rest of CPU usage goes to other places (which also can be optimized, but is completelly different issue :)).
    

Patch to implement required changes is attached. I'm working locally using Git, hopefully generated 'diff' is OK.

Notes:    

1. I've added 3 files into Base Utilities API

    LruCache   - my performance-efficient implementation of Last Recently Used items cache
    CachedFile - above mentioned CachedFile which uses LruCache to store its instances
    CachedFuri - cached file URIs and URLs. Self profiler shows that massive convertions between File <-> URL <-> URI take significant time in some algorithms.
    
2. Required addoptions are included, though - they are rather 'hacky'. I did not care much about most-correct-pulbic-API-friendly changes, however i do not expect problems in existing 3rd-party plugings because of API incompatibility. So - this is not a 'ready to use' patch, I expected someone from NB team will manully look through all changes and apply as needed (if needed).

Patch is made on-top of this revision http://hg.netbeans.org/main/rev/10cc566d91aa
Which i beleive is last commit for Release 8.2


3. there are some changes related to FileNaming - self-profiler also show nummber of hot points coming from this area. I just fixed the most annoying ones, but i would say it requires complete refactoring because of over-complicated logic and excesive usage of 'synchronized' methods which reduce multithreading capabilities of application.

    This is about files in 
        masterfs\src\org\netbeans\modules\masterfs\filebasedfs\naming\*
        masterfs\src\org\netbeans\modules\masterfs\filebasedfs\fileobjects\*

    Some specific points regarding
        masterfs\src\org\netbeans\modules\masterfs\filebasedfs\naming\FileNaming.java
    
    - There are no JavaDoc comments
        > what is 'getId();' ?
        > what is 'FileNaming rename(String name, ProvidedExtensions.IOHandler handler) throws IOException;' ?
        
      How actual implementation correspond to design?
            
            masterfs\filebasedfs\naming\NamingFactory.java
                public static FileNaming[] rename (...)

        -> masterfs\filebasedfs\naming\FileName.java (why is it returning 'this'?)

                @Override
                public FileNaming rename(String name, ProvidedExtensions.IOHandler handler) throws IOException {
                    boolean success = false;
                    final File f = getFile();

                    if (FileChangedManager.getInstance().exists(f)) {
                        ...
                        if (success) {
                            return NamingFactory.fromFile(getParent(), newFile, true);
                        }
                    }
                    return this;
                }

4. To support optimal performance with Lucene indexes one 'patch' is required there in version 3.5.0 used in NetBeans
   https://github.com/apache/lucene-solr.git -> lucene\src\java\org\apache\lucene\store\FSDirectory.java
   
          private static File getCanonicalPath(File file) throws IOException {
            return new File(file.getCanonicalPath());
          }

    replace with this implementation so that CachedFile provided by NB do not get lost in Lucene code
          private static File getCanonicalPath(File file) throws IOException {
            return file.getCanonicalFile();
          }


Relevant/affected issues:

Excessive calls to File.isDirectory() cause performance problems
https://netbeans.org/bugzilla/show_bug.cgi?id=250470   

Slow and Weird auto-suggest for Exception class names in New Breakpoint dialog
https://netbeans.org/bugzilla/show_bug.cgi?id=250470

'Heavy' CPU-intensive implementation of CachingArchiveProvider.toURI()
https://netbeans.org/bugzilla/show_bug.cgi?id=268292

Performance issue in BaseFileObj.getPath --> FileName.getName
https://netbeans.org/bugzilla/show_bug.cgi?id=268026

Performance issue: FileUtil.normalizedRef cache is cleaned too frequently
https://netbeans.org/bugzilla/show_bug.cgi?id=268027

Performance issue in RepositoryForBinaryQueryImpl.getJarMetadataCoordinatesIntern()
https://netbeans.org/bugzilla/show_bug.cgi?id=268045