ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Rees <d.ree...@usa.net>
Subject Re: [REVIEW/SUBMIT] Pattern-matching optimization
Date Sun, 01 Apr 2001 21:34:06 GMT
Let me prepend this with the general comment that I agree with
creating Pattern objects and your general changes from a design
perspective. But I am worried because this is such a core part of the
system. My belief is that for such a core piece that it is preferable
to accept a "uglier" solution that makes less changes than a cleaner
one that makes more. 

I wonder maybe if a simpler static method check for "simple filename"
would make more sense for Ant 1.X and this idea should be included
into the general new design of scanners and fileset in Ant2.


On Tue, 27 Mar 2001 17:14:03 -0500, Eric Siegerman wrote:

>On Tue, Mar 27, 2001 at 01:41:56PM -0800, David Rees wrote:
>> I would have concerns about moving the match code
>
>I left behind stub versions of the DirectoryScanner.matchPath and
>.matchPatternStart static methods, that delegate to the versions
>in Pattern.  I didn't leave behind a stub match() because that's
>not called from anyplace, and probably shouldn't be.
>
>> and especially about
>> changing includes/excludes in DirectoryScanner.
>
>I didn't move this stuff -- fields or methods.  There are a few
>minor changes like:
>	-    protected String[] includes;
>	+    protected Pattern[] includes;
>which requires:
>	-            this.includes = new String[includes.length];
>	+            this.includes = new Pattern[includes.length];
>and:
>	-                this.includes[i] = pattern;
>	+                this.includes[i] = new Pattern(pattern);
>
>but the inclusion/exclusion *logic* is completely unchanged; I
>only moved the stuff that, independent of any context, answers
>the question, "does filename F match pattern P?"
>
>> A lot of various
>> things in Ant reference it (as I discovered when I tried to move it
>> ;).
>
>FWIW, in order to make ant compile with my changes, the only
>other file that needed to be touched was types/ZipScanner.java,
>and that was again just trivial initialization stuff like the
>above (making it call "super.setIncludes()" instead of poking the
>"includes" field itself).
>

Did you check the optional tasks? Specifically VAJWorkspaceScanner and
FTP$FTPDirectoryScanner?

>> Couldn't you just apply your patch to the existing methods?
>
>No.  Or at least, not at all cleanly!  Without the refactoring,
>there's no place to store isSimpleFilename -- without some gory
>hack like keeping arrays of booleans parallel to "includes" and
>"excludes", which I agree with you would be very dangerous.
>

Is caching the value what saves your time? From what I see I _think_
you could just put this in a static method and still get your
performance improvements.

>> As a side note, I think a major performance improvement would come
>> from caching the pattern arrays. Right now DS parses the pattern
>> strings every time a match call is made.
>
>Quite possibly.  This would also be much easier/cleaner/safer to
>do in a factored-out Pattern class than in code that's jumbled in
>with DirectoryScanner...

On that we completely agree ;).

dave

Mime
View raw message