ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gilles Scokart" <gscok...@gmail.com>
Subject Re: svn commit: r687768 - in /ant/core/trunk: WHATSNEW src/main/org/apache/tools/ant/DirectoryScanner.java src/main/org/apache/tools/ant/types/selectors/PathPattern.java src/main/org/apache/tools/ant/types/selectors/SelectorUtils.java
Date Thu, 21 Aug 2008 14:28:51 GMT
Please, don't hesitate to comment this change.
In particular, I'm not sure where to place the PathPattern class.  I
currently put it in org/apache/tools/ant/types/selectors so that I
don't have to make the methods of SelectorUtils public, but I feel
like it is not the right package.  I'm worndering if something only
visible to the DirectoryScanner wouldn't be better.
WDYT ?

By the way, I also removed some explicit nulling.  I don't think they
offered any performance gain.  Did they?


2008/8/21  <gscokart@apache.org>:
> Author: gscokart
> Date: Thu Aug 21 07:18:59 2008
> New Revision: 687768
>
> URL: http://svn.apache.org/viewvc?rev=687768&view=rev
> Log:
> Enhance performance of DirectoryScanner by parsing the patterns only once.  Introduce
a PathPattern to do that. (# 44226)
>
> Added:
>    ant/core/trunk/src/main/org/apache/tools/ant/types/selectors/PathPattern.java   (with
props)
> Modified:
>    ant/core/trunk/WHATSNEW
>    ant/core/trunk/src/main/org/apache/tools/ant/DirectoryScanner.java
>    ant/core/trunk/src/main/org/apache/tools/ant/types/selectors/SelectorUtils.java
>
> Modified: ant/core/trunk/WHATSNEW
> URL: http://svn.apache.org/viewvc/ant/core/trunk/WHATSNEW?rev=687768&r1=687767&r2=687768&view=diff
> ==============================================================================
> --- ant/core/trunk/WHATSNEW (original)
> +++ ant/core/trunk/WHATSNEW Thu Aug 21 07:18:59 2008
> @@ -1,4 +1,4 @@
> -Changes from Ant 1.7.x TO current SVN version
> +eol-styleChanges from Ant 1.7.x TO current SVN version
>  =============================================
>
>  Changes that could break older environments:
> @@ -158,8 +158,6 @@
>    filter tokens
>    Bugzilla Report 44226.
>
> - * Enhance performance of Project.fireMessageLoggedEvent
> -   Bugzilla Report 45651.
>
>  Other changes:
>  --------------
> @@ -274,6 +272,9 @@
>    expression.
>    Bugzilla Report 45284
>
> + * Enhanced performance of Project.fireMessageLoggedEvent and DirectoryScanner
> +   Bugzilla Report 45651 & 45665
> +
>  * The package list location for offline links can now be specified as
>    an URL.
>    Bugzilla Report 28881
>
> Modified: ant/core/trunk/src/main/org/apache/tools/ant/DirectoryScanner.java
> URL: http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/DirectoryScanner.java?rev=687768&r1=687767&r2=687768&view=diff
> ==============================================================================
> --- ant/core/trunk/src/main/org/apache/tools/ant/DirectoryScanner.java (original)
> +++ ant/core/trunk/src/main/org/apache/tools/ant/DirectoryScanner.java Thu Aug 21 07:18:59
2008
> @@ -34,6 +34,7 @@
>  import org.apache.tools.ant.types.ResourceFactory;
>  import org.apache.tools.ant.types.resources.FileResource;
>  import org.apache.tools.ant.types.selectors.FileSelector;
> +import org.apache.tools.ant.types.selectors.PathPattern;
>  import org.apache.tools.ant.types.selectors.SelectorScanner;
>  import org.apache.tools.ant.types.selectors.SelectorUtils;
>  import org.apache.tools.ant.util.FileUtils;
> @@ -319,10 +320,8 @@
>      * <p>Gets lazily initialized on the first invocation of
>      * isIncluded or isExcluded and cleared at the end of the scan
>      * method (cleared in clearCaches, actually).</p>
> -     *
> -     * @since Ant 1.6.3
>      */
> -    private String[] includePatterns;
> +    private PathPattern[] includePatterns;
>
>     /**
>      * Array of all exclude patterns that contain wildcards.
> @@ -330,10 +329,8 @@
>      * <p>Gets lazily initialized on the first invocation of
>      * isIncluded or isExcluded and cleared at the end of the scan
>      * method (cleared in clearCaches, actually).</p>
> -     *
> -     * @since Ant 1.6.3
>      */
> -    private String[] excludePatterns;
> +    private PathPattern[] excludePatterns;
>
>     /**
>      * Have the non-pattern sets and pattern arrays for in- and
> @@ -1196,7 +1193,7 @@
>             return true;
>         }
>         for (int i = 0; i < includePatterns.length; i++) {
> -            if (matchPath(includePatterns[i], name, isCaseSensitive())) {
> +            if (includePatterns[i].matchPath(name, isCaseSensitive())) {
>                 return true;
>             }
>         }
> @@ -1296,7 +1293,7 @@
>             return true;
>         }
>         for (int i = 0; i < excludePatterns.length; i++) {
> -            if (matchPath(excludePatterns[i], name, isCaseSensitive())) {
> +            if (excludePatterns[i].matchPath(name, isCaseSensitive())) {
>                 return true;
>             }
>         }
> @@ -1696,18 +1693,17 @@
>      * @param patterns String[] of patterns.
>      * @since Ant 1.6.3
>      */
> -    private String[] fillNonPatternSet(Set set, String[] patterns) {
> +    private PathPattern[] fillNonPatternSet(Set set, String[] patterns) {
>         ArrayList al = new ArrayList(patterns.length);
>         for (int i = 0; i < patterns.length; i++) {
>             if (!SelectorUtils.hasWildcards(patterns[i])) {
>                 set.add(isCaseSensitive() ? patterns[i]
>                     : patterns[i].toUpperCase());
>             } else {
> -                al.add(patterns[i]);
> +                al.add(new PathPattern(patterns[i]));
>             }
>         }
> -        return set.size() == 0 ? patterns
> -            : (String[]) al.toArray(new String[al.size()]);
> +        return (PathPattern[]) al.toArray(new PathPattern[al.size()]);
>     }
>
>  }
>
> Added: ant/core/trunk/src/main/org/apache/tools/ant/types/selectors/PathPattern.java
> URL: http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/types/selectors/PathPattern.java?rev=687768&view=auto
> ==============================================================================
> --- ant/core/trunk/src/main/org/apache/tools/ant/types/selectors/PathPattern.java (added)
> +++ ant/core/trunk/src/main/org/apache/tools/ant/types/selectors/PathPattern.java Thu
Aug 21 07:18:59 2008
> @@ -0,0 +1,82 @@
> +/*
> + *  Licensed to the Apache Software Foundation (ASF) under one or more
> + *  contributor license agreements.  See the NOTICE file distributed with
> + *  this work for additional information regarding copyright ownership.
> + *  The ASF licenses this file to You under the Apache License, Version 2.0
> + *  (the "License"); you may not use this file except in compliance with
> + *  the License.  You may obtain a copy of the License at
> + *
> + *      http://www.apache.org/licenses/LICENSE-2.0
> + *
> + *  Unless required by applicable law or agreed to in writing, software
> + *  distributed under the License is distributed on an "AS IS" BASIS,
> + *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + *  See the License for the specific language governing permissions and
> + *  limitations under the License.
> + *
> + */
> +
> +package org.apache.tools.ant.types.selectors;
> +
> +/**
> + * Provides reusable path pattern matching.  PathPattern is preferable to equivalent
> + * SelectorUtils methods if you need to execute multiple matching with the same pattern
> + * because here the pattern itself will be parsed only once.
> + * @see SelectorUtils#matchPath(String, String)
> + * @see SelectorUtils#matchPath(String, String, boolean)
> + * @since 1.8
> + */
> +public class PathPattern {
> +
> +    private final String pattern;
> +    private final String tokenizedPattern[];
> +
> +    /**
> +    * Initialize the PathPattern by parsing it.
> +    * @param pattern The pattern to match against. Must not be
> +    *                <code>null</code>.
> +    */
> +    public PathPattern(String pattern) {
> +        this.pattern = pattern;
> +        this.tokenizedPattern = SelectorUtils.tokenizePathAsArray(pattern);
> +    }
> +
> +    /**
> +     * Tests whether or not a given path matches a given pattern.
> +     *
> +     * @param str     The path to match, as a String. Must not be
> +     *                <code>null</code>.
> +     *
> +     * @return <code>true</code> if the pattern matches against the string,
> +     *         or <code>false</code> otherwise.
> +     */
> +    public boolean matchPath(String str) {
> +        return SelectorUtils.matchPath(tokenizedPattern, str, true);
> +    }
> +
> +    /**
> +     * Tests whether or not a given path matches a given pattern.
> +     *
> +     * @param str     The path to match, as a String. Must not be
> +     *                <code>null</code>.
> +     * @param isCaseSensitive Whether or not matching should be performed
> +     *                        case sensitively.
> +     *
> +     * @return <code>true</code> if the pattern matches against the string,
> +     *         or <code>false</code> otherwise.
> +     */
> +    public boolean matchPath(String str, boolean isCaseSensitive) {
> +        return SelectorUtils.matchPath(tokenizedPattern, str, isCaseSensitive);
> +    }
> +
> +    /**
> +     * @return The pattern String
> +     */
> +    public String toString() {
> +        return pattern;
> +    }
> +
> +    public String getPattern() {
> +        return pattern;
> +    }
> +}
>
> Propchange: ant/core/trunk/src/main/org/apache/tools/ant/types/selectors/PathPattern.java
> ------------------------------------------------------------------------------
>    svn:eol-style = native
>
> Modified: ant/core/trunk/src/main/org/apache/tools/ant/types/selectors/SelectorUtils.java
> URL: http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/types/selectors/SelectorUtils.java?rev=687768&r1=687767&r2=687768&view=diff
> ==============================================================================
> --- ant/core/trunk/src/main/org/apache/tools/ant/types/selectors/SelectorUtils.java (original)
> +++ ant/core/trunk/src/main/org/apache/tools/ant/types/selectors/SelectorUtils.java Thu
Aug 21 07:18:59 2008
> @@ -38,7 +38,7 @@
>  */
>  public final class SelectorUtils {
>
> -    private static SelectorUtils instance = new SelectorUtils();
> +    private static final SelectorUtils instance = new SelectorUtils();
>     private static final FileUtils FILE_UTILS = FileUtils.getFileUtils();
>
>     /**
> @@ -144,6 +144,11 @@
>     /**
>      * Tests whether or not a given path matches a given pattern.
>      *
> +     * If you need to call this method multiple times with the same
> +     * pattern you should rather use PathPattern
> +     *
> +     * @see PathPattern
> +     *
>      * @param pattern The pattern to match against. Must not be
>      *                <code>null</code>.
>      * @param str     The path to match, as a String. Must not be
> @@ -153,12 +158,18 @@
>      *         or <code>false</code> otherwise.
>      */
>     public static boolean matchPath(String pattern, String str) {
> -        return matchPath(pattern, str, true);
> +        String[] patDirs = tokenizePathAsArray(pattern);
> +        return matchPath(patDirs, str, true);
>     }
>
>     /**
>      * Tests whether or not a given path matches a given pattern.
> +     *
> +     * If you need to call this method multiple times with the same
> +     * pattern you should rather use PathPattern
>      *
> +     * @see PathPattern
> +     *
>      * @param pattern The pattern to match against. Must not be
>      *                <code>null</code>.
>      * @param str     The path to match, as a String. Must not be
> @@ -172,22 +183,28 @@
>     public static boolean matchPath(String pattern, String str,
>                                     boolean isCaseSensitive) {
>         String[] patDirs = tokenizePathAsArray(pattern);
> +        return matchPath(patDirs, str, isCaseSensitive);
> +    }
> +
> +    /**
> +     * Core implementation of matchPath.  It is isolated so that it can be called from
> +     * PathPattern.
> +     */
> +    static boolean matchPath(String[] tokenizedPattern, String str, boolean isCaseSensitive)
{
>         String[] strDirs = tokenizePathAsArray(str);
>
>         int patIdxStart = 0;
> -        int patIdxEnd = patDirs.length - 1;
> +        int patIdxEnd = tokenizedPattern.length - 1;
>         int strIdxStart = 0;
>         int strIdxEnd = strDirs.length - 1;
>
>         // up to first '**'
>         while (patIdxStart <= patIdxEnd && strIdxStart <= strIdxEnd) {
> -            String patDir = patDirs[patIdxStart];
> +            String patDir = tokenizedPattern[patIdxStart];
>             if (patDir.equals("**")) {
>                 break;
>             }
>             if (!match(patDir, strDirs[strIdxStart], isCaseSensitive)) {
> -                patDirs = null;
> -                strDirs = null;
>                 return false;
>             }
>             patIdxStart++;
> @@ -196,9 +213,7 @@
>         if (strIdxStart > strIdxEnd) {
>             // String is exhausted
>             for (int i = patIdxStart; i <= patIdxEnd; i++) {
> -                if (!patDirs[i].equals("**")) {
> -                    patDirs = null;
> -                    strDirs = null;
> +                if (!tokenizedPattern[i].equals("**")) {
>                     return false;
>                 }
>             }
> @@ -206,21 +221,17 @@
>         } else {
>             if (patIdxStart > patIdxEnd) {
>                 // String not exhausted, but pattern is. Failure.
> -                patDirs = null;
> -                strDirs = null;
>                 return false;
>             }
>         }
>
>         // up to last '**'
>         while (patIdxStart <= patIdxEnd && strIdxStart <= strIdxEnd) {
> -            String patDir = patDirs[patIdxEnd];
> +            String patDir = tokenizedPattern[patIdxEnd];
>             if (patDir.equals("**")) {
>                 break;
>             }
>             if (!match(patDir, strDirs[strIdxEnd], isCaseSensitive)) {
> -                patDirs = null;
> -                strDirs = null;
>                 return false;
>             }
>             patIdxEnd--;
> @@ -229,9 +240,7 @@
>         if (strIdxStart > strIdxEnd) {
>             // String is exhausted
>             for (int i = patIdxStart; i <= patIdxEnd; i++) {
> -                if (!patDirs[i].equals("**")) {
> -                    patDirs = null;
> -                    strDirs = null;
> +                if (!tokenizedPattern[i].equals("**")) {
>                     return false;
>                 }
>             }
> @@ -241,7 +250,7 @@
>         while (patIdxStart != patIdxEnd && strIdxStart <= strIdxEnd) {
>             int patIdxTmp = -1;
>             for (int i = patIdxStart + 1; i <= patIdxEnd; i++) {
> -                if (patDirs[i].equals("**")) {
> +                if (tokenizedPattern[i].equals("**")) {
>                     patIdxTmp = i;
>                     break;
>                 }
> @@ -259,7 +268,7 @@
>             strLoop:
>                         for (int i = 0; i <= strLength - patLength; i++) {
>                             for (int j = 0; j < patLength; j++) {
> -                                String subPat = patDirs[patIdxStart + j + 1];
> +                                String subPat = tokenizedPattern[patIdxStart + j + 1];
>                                 String subStr = strDirs[strIdxStart + i + j];
>                                 if (!match(subPat, subStr, isCaseSensitive)) {
>                                     continue strLoop;
> @@ -271,8 +280,6 @@
>                         }
>
>             if (foundIdx == -1) {
> -                patDirs = null;
> -                strDirs = null;
>                 return false;
>             }
>
> @@ -281,9 +288,7 @@
>         }
>
>         for (int i = patIdxStart; i <= patIdxEnd; i++) {
> -            if (!patDirs[i].equals("**")) {
> -                patDirs = null;
> -                strDirs = null;
> +            if (!tokenizedPattern[i].equals("**")) {
>                 return false;
>             }
>         }
> @@ -507,7 +512,7 @@
>     /**
>      * Same as {@link #tokenizePath tokenizePath} but hopefully faster.
>      */
> -    private static String[] tokenizePathAsArray(String path) {
> +    /*package*/ static String[] tokenizePathAsArray(String path) {
>         String root = null;
>         if (FileUtils.isAbsolutePath(path)) {
>             String[] s = FILE_UTILS.dissect(path);
>
>
>



-- 
Gilles Scokart

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


Mime
View raw message