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 268045 - Performance issue in RepositoryForBinaryQueryImpl.getJarMetadataCoordinatesIntern()
Summary: Performance issue in RepositoryForBinaryQueryImpl.getJarMetadataCoordinatesIn...
Status: NEW
Alias: None
Product: ide
Classification: Unclassified
Component: Performance (show other bugs)
Version: Dev
Hardware: PC Windows 7
: P2 normal (vote)
Assignee: Tomas Hurka
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-15 16:20 UTC by NukemBy
Modified: 2016-09-15 16:20 UTC (History)
0 users

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description NukemBy 2016-09-15 16:20:05 UTC
To start, please take a look onto [https://netbeans.org/bugzilla/show_bug.cgi?id=65135#c40] to get idea of how expensive File.isFile() is.

Next, lets look onto this code:
- - - - - - - - - - - - - - - - - - - - - - - - - - - 
    private List<Coordinates> getJarMetadataCoordinatesIntern(File binaryFile) {        
        if (binaryFile == null || !binaryFile.exists() || !binaryFile.isFile()) {
            return null;
        }        
        synchronized (coorCache) {
            List<Coordinates> toRet = coorCache.get(binaryFile);
            if(toRet == null) {
                toRet = getJarMetadataCoordinates(binaryFile);         
                if(toRet != null) {
                    FileUtil.addFileChangeListener(binaryChangeListener, binaryFile);
                    coorCache.put(binaryFile, toRet);
                }                
            } 
            return toRet;
        }
    }
- - - - - - - - - - - - - - - - - - - - - - - - - - - 

Double-check for existence of file is already mentioned in #65135. Another issue is - expensive binaryFile.isFile() can be avoided at all by moving that verification under 'if(toRet == null)' - when that file is actually going to be used. 

'Removal' of the file should be traced by attached fileChangeListener - therefore verification for existence is not needed while file is in cache (i guess).

On more issue here - code forwarded to "toRet = getJarMetadataCoordinates(binaryFile);" performs double check for file existence once again. I would refactor code like following:

- - - - - - - - - - - - - - - - - - - - - - - - - - - 

    private List<Coordinates> getJarMetadataCoordinatesIntern(File binaryFile) {        
        if (binaryFile == null ) {
            return null;
        }

        synchronized (coorCache) {
            List<Coordinates> toRet = coorCache.get(binaryFile);
            if(toRet == null) {
                if( !binaryFile.isFile() )
                    return null;
                
                toRet = getJarMetadataCoordinates(binaryFile, false);         
                if(toRet != null) {
                    FileUtil.addFileChangeListener(binaryChangeListener, binaryFile);
                    coorCache.put(binaryFile, toRet);
                }                
            } 
            return toRet;
        }
    }
    
    public static List<Coordinates> getJarMetadataCoordinates(File binaryFile) {
        return getJarMetadataCoordinates(binaryFile, true);
    }
        
    public static List<Coordinates> getJarMetadataCoordinates(File binaryFile, boolean checkExists) {
        if (binaryFile == null || checkExists && !binaryFile.isFile()) {
            return null;
        }        

- - - - - - - - - - - - - - - - - - - - - - - - - - - 

That change brings negative effect of that issue [https://netbeans.org/bugzilla/show_bug.cgi?id=268028] close to zero.