Performance of EclipseResourceFileSystemAccess2.hasContentsChanged(IFile,Inputstream)

[cdietrich]

With Java 17 we see a 300% performance decrease in EclipseResourceFileSystemAccess2.hasContentsChanged(IFile,Inputstream) it looks like FileInputStream (ususally used by IFile.getContents) does no longer work together with BufferedInputStream as it did before. (Measurements taken on Linux)

• does windows and mac have the same problem?
• do other places that use the same pattern have the same problem?

for us it looks like using a buffer and then compare the buffers (Arrays.equals) shows better performance, also on Java 11 compared to the current used bytewise compare

[cdietrich]

possible patch using Arrays.equals with indices wont work in J8 @szarnekow any idea how to circumvent this? reflective guard?

[iloveeclipse]

it looks like FileInputStream (ususally used by IFile.getContents) does no longer work together with BufferedInputStream as it did before

Actually this is about using synchronized method in a loop which is now 3x slower thanks to disabled biased locking https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8231265

But nevertheless, even without this flag the old code was performing very poor compared with block wise array compare we have now, the difference is 10-15x faster with the patch also on Java 11.

[iloveeclipse]

possible patch using Arrays.equals with indices wont work in J8

:-(

One can use hand written array compare loop on Java 8...

[cdietrich]

@szarnekow @iloveeclipse maybe we can use guavas com.google.common.io.ByteStreams.read(InputStream, byte[], int, int) with extra check for leng 0

[iloveeclipse]

maybe we can use guavas com.google.common.io.ByteStreams.read(InputStream, byte[], int, int) with extra check for leng 0

Yes, would be OK. I've tested this, without much difference to previously proposed patch (and still 5x faster as in Java 11 without any patch):

    @Override
    protected final boolean hasContentsChanged(IFile file, InputStream newContent) {
        try {
            return isContentChanged(file.getContents(), newContent);
        } catch (CoreException e) {
            return true;
        }
    }

    public static boolean isContentChanged(InputStream oldContent, InputStream newStream) {
        // Since we don't know if the code may run in multiple threads, use thread local caches
        byte [] oldBytes = OLD_BYTES.get();
        byte [] newBytes = NEW_BYTES.get();
        boolean contentChanged;
        int newRead;
        int oldRead;
        try (InputStream oldStream = oldContent){
            do {
                oldRead = read(oldStream, oldBytes, 0, DEFAULT_BUFFER_SIZE);
                newRead = read(newStream, newBytes, 0, DEFAULT_BUFFER_SIZE);
                if(newRead != oldRead) {
                    contentChanged = true;
                    break;
                }
                contentChanged = !Arrays.equals(oldBytes, 0, oldRead, newBytes, 0, newRead);
                if(newRead < DEFAULT_BUFFER_SIZE) {
                    // EOF
                    break;
                }
            } while (!contentChanged);

        } catch (IOException e) {
            contentChanged = true;
        }
        return contentChanged;
    }

    private static int read(InputStream in, byte[] b, int off, int len) throws IOException {
        int total = 0;
        while (total < len) {
            int result = in.read(b, off + total, len - total);
            if (result == -1) {
                break;
            }
            total += result;
        }
        return total;
    }

[iloveeclipse]

possible patch using Arrays.equals with indices wont work in J8 @szarnekow any idea how to circumvent this? reflective guard?

Now with your announcement in https://www.eclipse.org/lists/cross-project-issues-dev/msg19411.html we shouldn't care about Java 8 anymore, right?

[cdietrich]

yes but the comment by sebastian applies to both

[cdietrich]

fixed in 2.30