From cafa34ca0878175f8c8ab1bf7fcfaba70c2b1368 Mon Sep 17 00:00:00 2001
From: "Jesse N. Glick" <jglick@apache.org>
Date: Wed, 3 Oct 2007 23:44:10 +0000
Subject: [PATCH] Various microoptimizations to reduce I/O load of common
 tasks, esp. no-op <javac> and <depend>. Many inner loops altered to make just
 1-2 system calls rather than 4-5. You can easily see how wasteful the
 previous code was, and find the culprits, by patching r/o java.io.File
 methods and adding to -Xbootclasspath/p (or use AspectJ). E.g.:

public boolean isDirectory() {
  System.err.println("isDirectory: " + this); if (Math.random() < .01) Thread.dumpStack();
  // as before...
}

Ant still makes an order of magnitude more system calls to do what seem like simple operations
than you would think necessary, but this patch should at least improve the situation.


git-svn-id: https://svn.apache.org/repos/asf/ant/core/trunk@581748 13f79535-47bb-0310-9956-ffa450edef68
---
 WHATSNEW                                      |  4 ++
 .../apache/tools/ant/DirectoryScanner.java    | 60 ++++++++++---------
 .../org/apache/tools/ant/taskdefs/Rmic.java   | 12 +++-
 .../ant/taskdefs/optional/depend/Depend.java  | 18 +++---
 .../ant/types/selectors/SelectorUtils.java    | 12 ++--
 .../apache/tools/ant/util/ResourceUtils.java  | 27 ++++-----
 .../tools/ant/util/SourceFileScanner.java     |  9 ++-
 7 files changed, 80 insertions(+), 62 deletions(-)

diff --git a/WHATSNEW b/WHATSNEW
index 5d8c24796..e947a0f61 100644
--- a/WHATSNEW
+++ b/WHATSNEW
@@ -196,6 +196,10 @@ Fixed bugs:
 
 Other changes:
 --------------
+
+* Various small optimizations speed up common tasks such as <javac> on large
+  filesets, reducing both I/O and CPU usage.
+
 * Profiling logger has been added with basic profiling capabilities.
 
 * <script> now has basic support for JavaFX scripts
diff --git a/src/main/org/apache/tools/ant/DirectoryScanner.java b/src/main/org/apache/tools/ant/DirectoryScanner.java
index 663fd7b2d..98f0c6722 100644
--- a/src/main/org/apache/tools/ant/DirectoryScanner.java
+++ b/src/main/org/apache/tools/ant/DirectoryScanner.java
@@ -1065,28 +1065,25 @@ public class DirectoryScanner
     protected void scandir(File dir, String vpath, boolean fast) {
         if (dir == null) {
             throw new BuildException("dir must not be null.");
-        } else if (!dir.exists()) {
-            throw new BuildException(dir + " doesn't exist.");
-        } else if (!dir.isDirectory()) {
-            throw new BuildException(dir + " is not a directory.");
         }
+        String[] newfiles = dir.list();
+        if (newfiles == null) {
+            if (!dir.exists()) {
+                throw new BuildException(dir + " doesn't exist.");
+            } else if (!dir.isDirectory()) {
+                throw new BuildException(dir + " is not a directory.");
+            } else {
+                throw new BuildException("IO error scanning directory '"
+                                         + dir.getAbsolutePath() + "'");
+            }
+        }
+        scandir(dir, vpath, fast, newfiles);
+    }
+    private void scandir(File dir, String vpath, boolean fast, String[] newfiles) {
         // avoid double scanning of directories, can only happen in fast mode
         if (fast && hasBeenScanned(vpath)) {
             return;
         }
-        String[] newfiles = dir.list();
-
-        if (newfiles == null) {
-            /*
-             * two reasons are mentioned in the API docs for File.list
-             * (1) dir is not a directory. This is impossible as
-             *     we wouldn't get here in this case.
-             * (2) an IO error occurred (why doesn't it throw an exception
-             *     then???)
-             */
-            throw new BuildException("IO error scanning directory '"
-                                     + dir.getAbsolutePath() + "'");
-        }
         if (!followSymlinks) {
             Vector noLinks = new Vector();
             for (int i = 0; i < newfiles.length; i++) {
@@ -1112,25 +1109,26 @@ public class DirectoryScanner
         for (int i = 0; i < newfiles.length; i++) {
             String name = vpath + newfiles[i];
             File file = new File(dir, newfiles[i]);
-            if (file.isDirectory()) {
+            String[] children = file.list();
+            if (children == null) { // probably file
+                if (isIncluded(name)) {
+                    accountForIncludedFile(name, file);
+                } else {
+                    everythingIncluded = false;
+                    filesNotIncluded.addElement(name);
+                }
+            } else { // dir
                 if (isIncluded(name)) {
-                    accountForIncludedDir(name, file, fast);
+                    accountForIncludedDir(name, file, fast, children);
                 } else {
                     everythingIncluded = false;
                     dirsNotIncluded.addElement(name);
                     if (fast && couldHoldIncluded(name)) {
-                        scandir(file, name + File.separator, fast);
+                        scandir(file, name + File.separator, fast, children);
                     }
                 }
                 if (!fast) {
-                    scandir(file, name + File.separator, fast);
-                }
-            } else if (file.isFile()) {
-                if (isIncluded(name)) {
-                    accountForIncludedFile(name, file);
-                } else {
-                    everythingIncluded = false;
-                    filesNotIncluded.addElement(name);
+                    scandir(file, name + File.separator, fast, children);
                 }
             }
         }
@@ -1158,6 +1156,12 @@ public class DirectoryScanner
             scandir(file, name + File.separator, fast);
         }
     }
+    private void accountForIncludedDir(String name, File file, boolean fast, String[] children) {
+        processIncluded(name, file, dirsIncluded, dirsExcluded, dirsDeselected);
+        if (fast && couldHoldIncluded(name) && !contentsExcluded(name)) {
+            scandir(file, name + File.separator, fast, children);
+        }
+    }
 
     private void processIncluded(String name, File file, Vector inc, Vector exc, Vector des) {
 
diff --git a/src/main/org/apache/tools/ant/taskdefs/Rmic.java b/src/main/org/apache/tools/ant/taskdefs/Rmic.java
index 0c7e9b1f3..471916306 100644
--- a/src/main/org/apache/tools/ant/taskdefs/Rmic.java
+++ b/src/main/org/apache/tools/ant/taskdefs/Rmic.java
@@ -524,9 +524,15 @@ public class Rmic extends MatchingTask {
                 scanDir(baseDir, files, adapter.getMapper());
             } else {
                 // otherwise perform a timestamp comparison - at least
-                scanDir(baseDir, new String[] {
-                        classname.replace('.', File.separatorChar)
-                        + ".class" }, adapter.getMapper());
+                String path = classname.replace('.', File.separatorChar) + ".class";
+                File f = new File(baseDir, path);
+                if (f.isFile()) {
+                    scanDir(baseDir, new String[] { path }, adapter.getMapper());
+                } else {
+                    // Does not exist, so checking whether it is up to date makes no sense.
+                    // Compilation will fail later anyway, but tests expect a certain output.
+                    compileList.add(classname);
+                }
             }
             int fileCount = compileList.size();
             if (fileCount > 0) {
diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/depend/Depend.java b/src/main/org/apache/tools/ant/taskdefs/optional/depend/Depend.java
index 14293a433..56b7c21aa 100644
--- a/src/main/org/apache/tools/ant/taskdefs/optional/depend/Depend.java
+++ b/src/main/org/apache/tools/ant/taskdefs/optional/depend/Depend.java
@@ -773,18 +773,21 @@ public class Depend extends MatchingTask {
      * Find the source file for a given class
      *
      * @param classname the classname in slash format.
+     * @param sourceFileKnownToExist if not null, a file already known to exist (saves call to .exists())
      */
-    private File findSourceFile(String classname) {
-        String sourceFilename = classname + ".java";
+    private File findSourceFile(String classname, File sourceFileKnownToExist) {
+        String sourceFilename;
         int innerIndex = classname.indexOf("$");
         if (innerIndex != -1) {
             sourceFilename = classname.substring(0, innerIndex) + ".java";
+        } else {
+            sourceFilename = classname + ".java";
         }
 
         // search the various source path entries
         for (int i = 0; i < srcPathList.length; ++i) {
             File sourceFile = new File(srcPathList[i], sourceFilename);
-            if (sourceFile.exists()) {
+            if (sourceFile.equals(sourceFileKnownToExist) || sourceFile.exists()) {
                 return sourceFile;
             }
         }
@@ -812,11 +815,10 @@ public class Depend extends MatchingTask {
         int length = filesInDir.length;
 
         int rootLength = root.getPath().length();
+        File sourceFileKnownToExist = null; // speed optimization
         for (int i = 0; i < length; ++i) {
             File file = new File(dir, filesInDir[i]);
-            if (file.isDirectory()) {
-                addClassFiles(classFileList, file, root);
-            } else if (file.getName().endsWith(".class")) {
+            if (filesInDir[i].endsWith(".class")) {
                 ClassFileInfo info = new ClassFileInfo();
                 info.absoluteFile = file;
                 String relativeName = file.getPath().substring(
@@ -824,8 +826,10 @@ public class Depend extends MatchingTask {
                     file.getPath().length() - ".class".length());
                 info.className
                     = ClassFileUtils.convertSlashName(relativeName);
-                info.sourceFile = findSourceFile(relativeName);
+                info.sourceFile = sourceFileKnownToExist = findSourceFile(relativeName, sourceFileKnownToExist);
                 classFileList.addElement(info);
+            } else {
+                addClassFiles(classFileList, file, root);
             }
         }
     }
diff --git a/src/main/org/apache/tools/ant/types/selectors/SelectorUtils.java b/src/main/org/apache/tools/ant/types/selectors/SelectorUtils.java
index f028c7953..91c368514 100644
--- a/src/main/org/apache/tools/ant/types/selectors/SelectorUtils.java
+++ b/src/main/org/apache/tools/ant/types/selectors/SelectorUtils.java
@@ -615,16 +615,16 @@ public final class SelectorUtils {
      */
     public static boolean isOutOfDate(Resource src, Resource target,
                                       long granularity) {
-        if (!src.isExists()) {
+        long sourceLastModified = src.getLastModified();
+        if (sourceLastModified == 0L) {
+            // Does not exist. Quicker than checking exists() again.
             return false;
         }
-        if (!target.isExists()) {
+        long targetLastModified = target.getLastModified();
+        if (targetLastModified == 0L) {
             return true;
         }
-        if ((src.getLastModified() - granularity) > target.getLastModified()) {
-            return true;
-        }
-        return false;
+        return (sourceLastModified - granularity) > targetLastModified;
     }
 
     /**
diff --git a/src/main/org/apache/tools/ant/util/ResourceUtils.java b/src/main/org/apache/tools/ant/util/ResourceUtils.java
index 4c085bb8d..860262e36 100644
--- a/src/main/org/apache/tools/ant/util/ResourceUtils.java
+++ b/src/main/org/apache/tools/ant/util/ResourceUtils.java
@@ -61,17 +61,6 @@ import org.apache.tools.ant.types.selectors.SelectorUtils;
  */
 public class ResourceUtils {
 
-    private static final class Outdated implements ResourceSelector {
-        private Resource control;
-        private long granularity;
-        private Outdated(Resource control, long granularity) {
-            this.control = control;
-            this.granularity = granularity;
-        }
-        public boolean isSelected(Resource r) {
-            return SelectorUtils.isOutOfDate(control, r, granularity);
-        }
-    }
     /** Utilities used for file operations */
     private static final FileUtils FILE_UTILS = FileUtils.getFileUtils();
 
@@ -144,7 +133,7 @@ public class ResourceUtils {
                                                             ResourceCollection source,
                                                             FileNameMapper mapper,
                                                             ResourceFactory targets,
-                                                            long granularity) {
+                                                            final long granularity) {
         if (source.size() == 0) {
             logTo.log("No sources found.", Project.MSG_VERBOSE);
             return Resources.NONE;
@@ -154,7 +143,7 @@ public class ResourceUtils {
 
         Union result = new Union();
         for (Iterator iter = source.iterator(); iter.hasNext();) {
-            Resource sr = (Resource) iter.next();
+            final Resource sr = (Resource) iter.next();
             String srName = sr.getName();
             srName = srName == null
                 ? srName : srName.replace('/', File.separatorChar);
@@ -178,8 +167,16 @@ public class ResourceUtils {
             }
             //find the out-of-date targets:
             Restrict r = new Restrict();
-            r.add(new And(new ResourceSelector[] {Type.FILE, new Or(
-                new ResourceSelector[] {NOT_EXISTS, new Outdated(sr, granularity)})}));
+            r.add(new ResourceSelector() {
+                public boolean isSelected(Resource target) {
+                    /* Extra I/O, probably wasted:
+                    if (target.isDirectory()) {
+                        return false;
+                    }
+                     */
+                    return SelectorUtils.isOutOfDate(sr, target, granularity);
+                }
+            });
             r.add(targetColl);
             if (r.size() > 0) {
                 result.add(sr);
diff --git a/src/main/org/apache/tools/ant/util/SourceFileScanner.java b/src/main/org/apache/tools/ant/util/SourceFileScanner.java
index fd3d8a988..e98698709 100644
--- a/src/main/org/apache/tools/ant/util/SourceFileScanner.java
+++ b/src/main/org/apache/tools/ant/util/SourceFileScanner.java
@@ -91,9 +91,12 @@ public class SourceFileScanner implements ResourceFactory {
         this.destDir = destDir;
         Vector v = new Vector();
         for (int i = 0; i < files.length; i++) {
-            File src = FILE_UTILS.resolveFile(srcDir, files[i]);
-            v.addElement(new Resource(files[i], src.exists(),
-                                      src.lastModified(), src.isDirectory()));
+            final String name = files[i];
+            v.addElement(new FileResource(srcDir, name) {
+                public String getName() {
+                    return name;
+                }
+            });
         }
         Resource[] sourceresources = new Resource[v.size()];
         v.copyInto(sourceresources);