ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Bodewig <bode...@apache.org>
Subject Re: cvs commit: ant/src/testcases/org/apache/tools/ant/taskdefs SyncTest.java
Date Tue, 09 Nov 2004 16:02:22 GMT
On Tue, 9 Nov 2004, Dominique Devienne <DDevienne@lgc.com> wrote:

> I'm worried about the performance of this change Stefan.

I understand that.

> I'm sure I'm probably not understanding your change correctly, so I
> guess I need an explanations for how your change works.

Basically, if I want to be able to keep some stuff in the target
directory, I'll need a way to exclude them from the purge process and
thus using the full power of DirectoryScanner seemed to be the natural
choice.

> 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?

Yes.

> I believe toArray doesn't care if the array provided is larger than
> needed.

This I am not sure about - and I worked on my iBook with only two JDKs
to test as opposed to my Linux desktop with seven 8-)

I wanted to be save here first.

>>   +        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.

Most time spent in DirectoryScanner is file system scanning AFAIU.
Antoine's changes improved performance for large lists of include
patterns by avoiding scans of directories completely.  Since the
original code had to scan the directories as well, I don#t think the
impact will be significant (though I agree it will be slower).

> 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) {

OK.  The reason is that we want to delete them depth first - you knew
that 8-)

Stefan

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


Mime
View raw message