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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
