ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dominique Devienne" <DDevie...@lgc.com>
Subject RE: cvs commit: ant/src/testcases/org/apache/tools/ant/taskdefs SyncTest.java
Date Tue, 09 Nov 2004 15:06:42 GMT
I'm worried about the performance of this change Stefan. I'm sure I'm
probably not understanding your change correctly, so I guess I need an
explanations for how your change works.

As my inline comments below state, I'm concerned about the possible
slowdown on large syncs by using DirectoryScanner. Thanks, --DD

> -----Original Message-----
> bodewig     2004/11/09 02:19:52
> 
>   Modified:    src/main/org/apache/tools/ant/taskdefs Sync.java
>                src/main/org/apache/tools/ant/types PatternSet.java
>                src/testcases/org/apache/tools/ant BuildFileTest.java
>   Added:       src/etc/testcases/taskdefs sync.xml
>                src/testcases/org/apache/tools/ant/taskdefs
SyncTest.java
>   Log:
>   Preparations for PR: 21832, in particular unit tests and some
>   refactorings for <sync>
> 
>   Changes to Sync:
>   * use a DirectoryScanner to find orphaned files instead of the
>     recursive algorithm.

>   +    private int[] removeOrphanFiles(Set nonOrphans, File toDir) {
>   +        int[] removedCount = new int[] {0, 0};
>   +        DirectoryScanner ds = new DirectoryScanner();
>   +        ds.setBasedir(toDir);
>   +        Set s = new HashSet(nonOrphans);
>   +        s.add("");
>   +        String[] excls = (String[]) s.toArray(new
String[s.size()]);

The nonOrphans set contains all files to be synced, i.e. it can be quite
large. Instead of taking a copy of the Set just to add the empty string,
couldn't we instead dimension the String array to nonOrphans.size() + 1,
and add the empty string at the end? I believe toArray doesn't care if
the array provided is larger than needed. Avoid the Set copy.

>   +        ds.setExcludes(excls);
>   +        ds.scan();

I guess this is where I'm the most concerned. As I've written above, the
nonOrphans Set will be quite large for large syncs, and even though I
know Antoine optimized DirectoryScanner a lot, I'm doubtful a scanner
with thousands of excludes as fast as a lookup in a Set, as the previous
implementation was.

>   +        String[] files = ds.getIncludedFiles();

This one's minor, but the previous impl also didn't require the
intermediate arrays for files and directories, as it was deleting things
as it went.

>   +        for (int i = 0; i < files.length; i++) {
>   +            File f = new File(toDir, files[i]);
>   +            log("Removing orphan file: " + f, Project.MSG_DEBUG);
>   +            f.delete();
>   +            ++removedCount[1];
>   +        }
>   +        String[] dirs = ds.getIncludedDirectories();

Maybe a comment as to why the for loop operates back to front would be
helpful here.

>   +        for (int i = dirs.length - 1 ; i >= 0 ; --i) {
>   +            File f = new File(toDir, dirs[i]);
>   +            log("Removing orphan directory: " + f,
Project.MSG_DEBUG);
>   +            f.delete();
>   +            ++removedCount[0];
>            }
>            return removedCount;
>        }

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Mime
View raw message