JDK-8153229 : JavacFiler.checkFileReopening drowns in exceptions after Modular Runtime Images change
  • Type: Bug
  • Component: tools
  • Sub-Component: javac
  • Affected Version: 9
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • Submitted: 2016-03-31
  • Updated: 2016-12-20
  • Resolved: 2016-12-12
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 9
9 b149Fixed
Related Reports
Relates :  
Relates :  
Relates :  
Description
The easiest way to reproduce is to build some annotation-processing-heavy project, like JMH:

$ hg clone http://hg.openjdk.java.net/code-tools/jmh/ jmh
$ cd jmh
$ mvn clean install -pl jmh-core-it -DskipTests

Time to compile:
  jdk9b111: 18.979 s
  8u66: 6.426 s

Massive regression! Profiling jdk9b111 case yields this very hot branch:
 |  +-  10.080   (9%)    org.openjdk.jmh.generators.annotations.APGeneratorDestinaton.newClass(java.lang.String)
 |  |  +-  10.060   (9%)    com.sun.tools.javac.processing.JavacFiler.createSourceFile(java.lang.CharSequence, javax.lang.model.element.Element[])
 |  |  |  +-  10.060   (9%)    com.sun.tools.javac.processing.JavacFiler.createSourceOrClassFile(boolean, java.lang.String)
 |  |  |    +-  10.030   (9%)    com.sun.tools.javac.processing.JavacFiler.checkFileReopening(javax.tools.FileObject, boolean)
 |  |  |    |  +-  9.990   (8%)    com.sun.tools.javac.file.JavacFileManager.isSameFile(javax.tools.FileObject, javax.tools.FileObject)
 |  |  |    |  |  +-  9.990   (8%)    com.sun.tools.javac.file.PathFileObject.isSameFile(com.sun.tools.javac.file.PathFileObject)
 |  |  |    |  |    +-  9.990   (8%)    java.nio.file.Files.isSameFile(java.nio.file.Path, java.nio.file.Path)
 |  |  |    |  |      +-  9.990   (8%)    sun.nio.fs.UnixFileSystemProvider.isSameFile(java.nio.file.Path, java.nio.file.Path)
 |  |  |    |  |        +-  7.080   (6%)    sun.nio.fs.UnixFileAttributes.get(sun.nio.fs.UnixPath, boolean)
 |  |  |    |  |        |  +-  7.080   (6%)    sun.nio.fs.UnixNativeDispatcher.stat(sun.nio.fs.UnixPath, sun.nio.fs.UnixFileAttributes)
 |  |  |    |  |        |    +-  6.700   (6%)    sun.nio.fs.UnixNativeDispatcher.stat0(long, sun.nio.fs.UnixFileAttributes)
 |  |  |    |  |        |    |  +-  2.750   (2%)    sun.nio.fs.UnixException.<init>(int)
 |  |  |    |  |        |    |    +-  2.750   (2%)    java.lang.Exception.<init>()
 |  |  |    |  |        |    |      +-  2.750   (2%)    java.lang.Throwable.<init>()
 |  |  |    |  |        |    |        +-  2.740   (2%)    java.lang.Throwable.fillInStackTrace()
 |  |  |    |  |        |    |          +-  2.740   (2%)    java.lang.Throwable.fillInStackTrace(int)

The relevant code is:

    private void checkFileReopening(FileObject fileObject, boolean addToHistory) throws FilerException {
        for (FileObject veteran : fileObjectHistory) {
            if (fileManager.isSameFile(veteran, fileObject)) {
                if (lint)
                    log.warning("proc.file.reopening", fileObject.getName());
                throw new FilerException("Attempt to reopen a file for path " + fileObject.getName());
            }
        }
        if (addToHistory)
            fileObjectHistory.add(fileObject);
    }

In other words, when doing isSameFile, we are stat0-ing the arguments, get the -1 error from stat if file does not exist, which throws an exception from native back to NIO to handle. Since checkFileReopening does the linear search through the history, if we *did not* yet created the fileObject, it does not exist, and we are getting all the exceptions for each comparison.

A trivial change, adding the exists check in checkFileReopening:

    private void checkFileReopening(FileObject fileObject, boolean addToHistory) throws FilerException {
        if (fileObject instanceof PathFileObject) {
            if (!Files.exists(((PathFileObject) fileObject).getPath())) {
                if (addToHistory)
                    fileObjectHistory.add(fileObject);
                return;
            }
        }

...drops the compilation time back to almost normal levels: 7.946 s
Comments
Jan is correct that the proposed fix to do instanceof PathFileObject is unacceptable, since there is no guarantee that PathFileObject is involved at all. A custom file manager could be using fully custom file objects. Therefore, we do have to do through JavaFileManager.isSameFile. The javac impl of file manager does delegate isSameFile to PathFileObject.isSameFile, which does default to using Files.isSameFile, but it could certainly be overriden in selected cases, where more information is available, and maybe even use the cached realPath if necessary (available via the parent fileManager fsInfo.)
06-05-2016

This issue seems to blow up jcstress compilation time from 2 minutes to 8 minutes. This is a massive regression, please fix this in 9.
02-05-2016

The problem with Instanceof PathFileObject is that a custom file manager may be wrapping the standard PathFileObject with a ForwardingJavaFileObject, and this check will fail then. I wonder if it would be enough to keep the real path in PathFileObject, and then just do Path.equals on the real paths rather than go to Files.isSameFile. (JDK 8 did File.toAbsolutePath().equals(other.toAbsolutePath()), so real path + equals would seem to be consistent with that).
01-04-2016